Lowercase bool variables values for consistency#16
Conversation
arm4b
left a comment
arm4b
left a comment
There was a problem hiding this comment.
Thanks for finding all the places! 👍
The change looks good overall,
Looks like this plugin does something special with the converters, so try to find out the logic there how the conversion applies and at which step. As discussed, please test the helm plugin manually if lowercasing would work OK and doesn't break the normal plugin execution.
@LeoDiazL Please provide your findings in the comments so this PR could be merged.
arm4b
changed the title
| fi | ||
| else | ||
| if [[ "${FETCH_KUBECONFIG}" == "False" ]]; then | ||
| if [[ "${FETCH_KUBECONFIG}" == "false" ]]; then |
There was a problem hiding this comment.
From the conversation, this one is still coming uppercased.
The ENV var default is defined in the
Lines 69 to 71 in ecac7a1
The context is that Helm chart is using
Line 11 in ecac7a1
meaning bool var parsing is happening inside the Helm chart with some
bash.
We'll need to ideally find the place where it's happening.
There was a problem hiding this comment.
Looks like in the helm pack shyaml tool is used for parsing the plugin schema
which probably converts bool values to the uppercase.
Worth the debugging here:
helm/scripts/bitops-config/converters/boolean.sh
Lines 19 to 20 in 097ad8a
helm/scripts/bitops-config/convert-schema.sh
Line 129 in ecac7a1
There was a problem hiding this comment.
The alternative approach is to throw away all this bash insanity https://github.com/bitops-plugins/helm/blob/main/scripts/bitops-config/convert-schema.sh and switch to (now fixed) core_schema_parsing: true
Line 11 in ecac7a1
|
|
3 participants
Converted to lower-case and added validations to avoid issues.